Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LANG] Generalize compute to tensor region #1476

Merged
merged 8 commits into from
Oct 6, 2018

Conversation

ZihengJiang
Copy link
Contributor

Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from others in the community.

Another way for tensor intrinsic

@tqchen
Copy link
Member

tqchen commented Jul 24, 2018

@ZihengJiang can you first open an RFC issue discussing the proposal and designs, examples, then link to the PR?

@xqdan
Copy link
Contributor

xqdan commented Jul 30, 2018

We have some operators based on tensor op, also we've done lots of testing for this api, could this be merged into master ASAP? @tqchen @ZihengJiang

@xqdan
Copy link
Contributor

xqdan commented Jul 30, 2018

@ZihengJiang could you please fix cpp lint first?

@tqchen tqchen self-assigned this Aug 1, 2018
@ZihengJiang ZihengJiang requested a review from yzhliu as a code owner August 6, 2018 02:47
@ZihengJiang ZihengJiang force-pushed the tensor_op branch 2 times, most recently from 27dbfbe to f892571 Compare August 6, 2018 04:10
# test_tensor_reduce()
test_tensor_tensor_op()
# test_tensor_scan()
# test_scan_multi_out()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to enable these cases


Operation TensorOpNode::make(std::string name,
std::string tag,
Array<IterVar> axis,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const reference?

@tqchen tqchen added this to the v0.5 milestone Aug 21, 2018
@ZihengJiang ZihengJiang added status: need review and removed status: need RFC need RFC discussion labels Sep 23, 2018
@tqchen
Copy link
Member

tqchen commented Sep 28, 2018

@were @yzhliu @merrymercy can you help review this PR ?

@@ -182,6 +182,80 @@ class PlaceholderOpNode : public OperationNode {
TVM_DECLARE_NODE_TYPE_INFO(PlaceholderOpNode, OperationNode);
};

class TensorComputeOpNode : public OperationNode {
public:
Array<IterVar> axis;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document the fields

v->Visit("reduce_axis", &reduce_axis);
v->Visit("sch_ndim", &sch_ndim);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schedulable_ndim is fine,

@@ -186,12 +186,14 @@ class TensorComputeOpNode : public OperationNode {
public:
Array<IterVar> axis;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, document each field

@@ -154,6 +155,7 @@ def intrin_func(ins, outs):

s = tvm.create_schedule(C.op)
stmt = tvm.lower(s, [A, B, C], simple_mode=True)
print(stmt)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove print

@tqchen tqchen added status: need update need update based on feedbacks and removed status: need review labels Oct 4, 2018
@ZihengJiang ZihengJiang changed the title Tensor OP [LANG] Generalize compute to tensor region Oct 4, 2018
@tqchen tqchen merged commit b90620e into apache:master Oct 6, 2018
@tqchen
Copy link
Member

tqchen commented Oct 6, 2018

Thanks @ZihengJiang @xqdan @yzhliu @zhiics this is now merged

@ZihengJiang ZihengJiang deleted the tensor_op branch October 6, 2018 04:41
FrozenGene pushed a commit to FrozenGene/tvm that referenced this pull request Dec 27, 2018
@ZihengJiang ZihengJiang mentioned this pull request Feb 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepted status: need update need update based on feedbacks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants